DRAFT: [EAPQE-2410] Add test to verify AJP Listener allowed-request-attributes-pattern #120
Conversation
| }; | ||
|
|
||
| before(() => { | ||
| cy.startWildflyContainer() |
There was a problem hiding this comment.
startWildflyContainer could be extended to accept parameters for standalone.sh, so that the test could be executed for the preview level.
There was a problem hiding this comment.
Yes, you could define optional additional start parameter to: https://github.com/istraka/berg/blob/EAPQE-2410/packages/testsuite/cypress.config.ts#L19
and here just use it (additionalStartParameter="--stability=preview")
what do you think @OndrejKotek @istraka?
There was a problem hiding this comment.
@OndrejKotek added. I didn't add the parameter to the task, because I assume I would have to modify tests (setting the parameter). Now I control it via environment variable.
There was a problem hiding this comment.
@istraka i don't like this approach for two reasons. You have to edit job automation and you don't see the test require a preview mode. could you please introduce a new parameter which you will define when the server is stared.
You can find inspiration in hostMode implemented for OIDC. This is almost same case.
There was a problem hiding this comment.
IIUC this solution would always start wildfly in preview stability mode. However we want to test the feature NOT in preview mode (in the future). With the proposed solution we wouldn't check default one, which we intend to do so. Once WFLY-19808 is resolved I would have to submit yet another PR. I'd like to keep it to 1:)
There was a problem hiding this comment.
My question is: do we want to have tests in the TS that runs in preview mode? And what if they are promoted to higher stability level? Should the test change?
There was a problem hiding this comment.
IIUC this solution would always start wildfly in preview stability mode
I don't think. You can define the default value to false and only in your case you can set it to true. so only your test you will start server in preview mode.
Like in case hostMode. Default value is network mode so when you will not use any parameter it will started in network mode. And only OIDC test define the parameter useNetworkHostMode: true so only when the parameter is present then the server will start a server in hostMode.
However we want to test the feature NOT in preview mode (in the future)
When this will be changed there have to be some JIRA to track this change. Because the documentation have to be also update etc. So also these test have to update to remove the parameter for preview mode.
Now you still adding the required subsystem manually and when it will be promoted to regular feature you will have to remove this manual configuration.
You will have to update a tests in every case. So why not to do property and explicitly define the preview mode is active?
do we want to have tests in the TS that runs in preview mode? And what if they are promoted to higher stability level? Should the test change?
No we wont start server in preview mode only for your test.
And what if they are promoted to higher stability level? Should the test change?
Only the parameter from your test will be removed.
There was a problem hiding this comment.
I am afraid of forgotten test running in preview stability mode. We have discussed this and agreed we should not do that and I'd like to wait with this PR for WFLY-19808.
We are waiting for the feature to be promoted to the default stability level WFLY-19808. EAP7-1836 is blocked by EAPDOC-2314.
IIUC we need the RFE to be in a sprint for WFLY-19808. But Bartosz has submitted the MR, and if it gets merged....well, we can merge this.
| }; | ||
|
|
||
| before(() => { | ||
| cy.startWildflyContainer() |
There was a problem hiding this comment.
Yes, you could define optional additional start parameter to: https://github.com/istraka/berg/blob/EAPQE-2410/packages/testsuite/cypress.config.ts#L19
and here just use it (additionalStartParameter="--stability=preview")
what do you think @OndrejKotek @istraka?
...ages/testsuite/cypress/e2e/undertow/test-configuration-subsystem-undertow-ajp-listener.cy.ts
Show resolved
Hide resolved
| cy.get(serverSelectors.ajpListenerItem).click(); | ||
| }); | ||
|
|
||
| it("Test AJP Listener: allowed request attributes pattern", () => { |
There was a problem hiding this comment.
Do test also negatived scenario? When is no possible to define AJP_ALLOWED_REQUEST_ATTRIBUTES_PATTERN
What a user see and what happened when a preview mode is not allowed?
There was a problem hiding this comment.
If a preview mode is not allowed, the parameter is simply not there. However it should be visible all the time - once it is in default stability level.
I guess there is no negative scenario - it is a pattern. Only invalid value is invalid patter (i.e. (.*), but this is parsed during service startup (after you click on the reload) and the WEB UI doesn't check it. Same for the CLI op - it fails to laod after the reload.
There was a problem hiding this comment.
It could be a negative scenarios when a user fill a wrong expression.
There was a problem hiding this comment.
What would you expect in such case?
There was a problem hiding this comment.
I don't know. You are working on the RFE and you should know how does it behave when it is wrongly configured.
There was a problem hiding this comment.
I would expect some notification which say the expression is wrong.
There was a problem hiding this comment.
As i understand. You can configure a wrong expression without any warning then restart a server and then it will not start again. So you have a dead server which you can't reconfigure back because it is offline now. So you have to manually update the standalone.xml to revert you last change.
I don't know if it is possible to check it in advance. I am just asking.
There was a problem hiding this comment.
We have discussed this offline. Reload produce an error - the service won't start because of wrong expression. Console is still up and you can edit the expression (CLI or Web console).
istraka
force-pushed
the
EAPQE-2410
branch
2 times, most recently
from
f9e24d6
to
071d8bf
Compare
istraka
changed the title
The feature is in
previewmode, waiting on https://issues.redhat.com/browse/WFLY-19808Anyway, test is ready for a review, if you want to run it, configure container to start with
-Djboss.stability=preview